Skip to content

Fix rollback in auto-commit mode and handle numeric autocommit responses - #2

Closed
vikrantpuppala wants to merge 21 commits into
mainfrom
vpuppala/transaction-rollback-autocommit-fix
Closed

Fix rollback in auto-commit mode and handle numeric autocommit responses#2
vikrantpuppala wants to merge 21 commits into
mainfrom
vpuppala/transaction-rollback-autocommit-fix

Conversation

@vikrantpuppala

Copy link
Copy Markdown
Owner

Summary

  • rollback() now throws DatabricksTransactionException when called while the connection is in auto-commit mode (no active transaction), aligning behavior with the Simba JDBC driver
  • fetchAutoCommitStateFromServer() now accepts both "1"/"0" and "true"/"false" responses from the SET AUTOCOMMIT query, since different server implementations return different formats
  • Updated E2E and unit tests to reflect the corrected behavior

Context

Testing the transaction E2E tests against the Simba DatabricksJDBC42.jar driver (with ignoretransactions=0) revealed two behavioral differences:

  1. connection.rollback() in auto-commit mode should throw an exception, not silently succeed
  2. SET AUTOCOMMIT query returns "1"/"0" rather than "true"/"false" depending on the server

Test plan

  • Unit tests pass: DatabricksConnectionTest (42/42 pass)
  • E2E tests: TransactionTests and ExplicitTransactionStatementTests against staging warehouse

🤖 Generated with Claude Code

sreekanth-db and others added 21 commits January 10, 2026 01:27
## Description
<!-- Provide a brief summary of the changes made and the issue they aim
to address.-->
Added query tags to telemetry

## Testing
<!-- Describe how the changes have been tested-->
Tested with real workspace in both cases: when query tags are present /
not present. Behaviour is working as expected.

## Additional Notes to the Reviewer
<!-- Share any additional context or insights that may help the reviewer
understand the changes better. This could include challenges faced,
limitations, or compromises made during the development process.
Also, mention any areas of the code that you would like the reviewer to
focus on specifically. -->

NO_CHANGELOG=true

Signed-off-by: Sreekanth Vadigi <sreekanth.vadigi@databricks.com>
## Problem

Custom user agent from useragententry parameter wasn't included in
connector service HTTP requests for feature flag retrieval.

 ## Root Cause

Method execution order issue in UserAgentManager.setUserAgent() - custom
user agent was set AFTER the connector service request was made.

 ## Solution

Reordered the method to set custom user agent before calling
getClientUserAgent() (which triggers feature flag fetch).

## Testing

  - Added testCustomUserAgentIncludedBeforeClientTypeEvaluation() test
  
  NO_CHANGELOG=true

---------

Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
…action preview (databricks#1176)

## Summary
- Changes default value of `IgnoreTransactions` parameter from `0` to
`1`, making transactions disabled by default
- Updates `supportsTransactions()` to respect the `IgnoreTransactions`
flag, returning `false` when transactions are ignored (default) and
`true` when explicitly enabled
- Adds test case for when transactions are explicitly enabled via
`IgnoreTransactions=0`

## Background
The multi-statement transaction feature is currently in private preview
for limited workspaces. When BI tools (Tableau, Power BI, DBeaver)
detect transaction support via `supportsTransactions()`, they
automatically use transaction methods, causing failures for customers
not enrolled in the preview.

This change prevents unexpected failures for non-preview customers while
allowing preview participants to opt-in by explicitly setting
`IgnoreTransactions=0` in their connection string.

## Migration Path
- **Non-preview customers**: No action required - transactions are now
disabled by default
- **Preview participants**: Set `IgnoreTransactions=0` in connection
string to enable transaction support
- **GA migration**: When multi-statement transactions reach GA, flip the
default back to `0`

## Test plan
- [ ] Verify existing tests pass
- [ ] Verify default connection returns `supportsTransactions() = false`
- [ ] Verify connection with `IgnoreTransactions=0` returns
`supportsTransactions() = true`
- [ ] Verify transaction methods (`setAutoCommit`, `commit`, `rollback`)
are no-ops by default

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
## Description
Bump version to 3.1.1
…ricks#1177)

### Problem
When executing queries that return 0 rows (e.g., `WHERE 1=0`), complex
types (ARRAY, MAP, STRUCT) showed only generic type names instead of
detailed type information:

  **Before:**
  - `ARRAY` instead of `ARRAY<INT>`
  - `MAP` instead of `MAP<STRING,STRING>`
  - `STRUCT` instead of `STRUCT<field: TYPE>`

  **After:**
  - Detailed type information is correctly preserved for all row counts

  ### Root Cause
In `AbstractArrowResultChunk.java`, Arrow field metadata was only
extracted inside the `while(arrowStreamReader.loadNextBatch())` loop.
For queries with 0 rows, no batches are loaded, so the loop never
executes and metadata is never extracted.

**Code location:**
`/src/main/java/com/databricks/jdbc/api/impl/arrow/AbstractArrowResultChunk.java:338-359`

  ### Solution
Extract metadata from `VectorSchemaRoot` immediately after obtaining it,
**before** the `loadNextBatch()` loop.

The Arrow IPC format always sends the schema message first (before any
record batches), so field metadata is available even when there are 0
rows. `VectorSchemaRoot` contains field vectors with metadata regardless
of row count.

  **Key changes:**
  1. Moved metadata extraction from inside the while loop to before it
2. Added defensive null checks for `VectorSchemaRoot` and field vectors
  3. Added debug logging to track metadata extraction

  ### Testing

  #### Unit Test Coverage
- ✅ Added `testMetadataExtractionWithZeroRows()` to
`ArrowResultChunkTest`
  - ✅ Verifies Arrow field metadata is extracted correctly with 0 rows
  - ✅ Tests complex types: `ARRAY<INT>`, `MAP<STRING,STRING>`
  - ✅ All 2,693 unit tests pass

  #### Manual Verification
  Tested with queries returning 0 rows:
  ```sql
  SELECT array_col, map_col, struct_col
  FROM table
  WHERE 1=0

  Result: Metadata now correctly shows detailed type information

  Impact

  - Scope: Both SQL Exec API and Thrift Server (shared code path)
- Risk: Low - backward compatible change, only affects metadata
extraction timing
  - Benefits:
    - Fixes schema discovery for WHERE 1=0 pattern
    - Improves metadata availability for empty result sets
    - Aligns with Arrow IPC specification behavior

  Additional Context

- Arrow IPC specification guarantees schema is sent before record
batches
- VectorSchemaRoot.getFieldVectors() is available immediately after
ArrowStreamReader.getVectorSchemaRoot()
- No performance impact: metadata extraction is now done once upfront
instead of conditionally on first batch

---------

Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
## Description
<!-- Provide a brief summary of the changes made and the issue they aim
to address.-->
This PR introduces lazy loading support for inline Arrow results to
improve memory efficiency when handling large result sets.

Previously, InlineChunkProvider would eagerly fetch all arrow batches
upfront when results had hasMoreRows = true, which could lead to memory
issues with large datasets. This change splits the handling into two
separate paths:
1. Lazy path (new): For Thrift-based inline Arrow results (when
ARROW_BASED_SET is returned), we now use LazyThriftInlineArrowResult
which fetches arrow batches on-demand as the client iterates through
rows. This is similar to how LazyThriftResult works for columnar data.
2. Remote path (existing): For URL-based Arrow results (URL_BASED_SET),
we continue using ArrowStreamResult with RemoteChunkProvider which
downloads chunks from cloud storage.

The InlineChunkProvider is now only used for SEA results with JSON_ARRAY
format and INLINE disposition (contain all data inline {no hasMoreRows
flag set}).

This will reduce memory consumption and improve performance when dealing
with large inline Arrow result sets similar to
databricks#975.

## Testing
<!-- Describe how the changes have been tested-->
- Unit tests
- Integration tests
- Manual testing

## Additional Notes to the Reviewer
<!-- Share any additional context or insights that may help the reviewer
understand the changes better. This could include challenges faced,
limitations, or compromises made during the development process.
Also, mention any areas of the code that you would like the reviewer to
focus on specifically. -->
Bypassing an existing failure on CI/CD because of databricks@3e4f21c
…istency (databricks#1182)

## Summary
This PR adds TIMESTAMP_NTZ normalization in the Thrift path to ensure
consistent metadata behavior across both SEA and Thrift API paths.

## Background
PR databricks#1177 moved Arrow metadata extraction earlier in the processing
pipeline, which exposed an inconsistency: the Thrift path started
returning the correct "TIMESTAMP_NTZ" from server metadata, while the
SEA path was already normalizing it to "TIMESTAMP" for backward
compatibility.

## Changes
- Added TIMESTAMP_NTZ → TIMESTAMP normalization in
`DatabricksResultSetMetaData.java` Thrift constructor (lines 205-208)
- This brings Thrift path behavior in line with existing SEA path
normalization
- Fixes test failure in
`PreparedStatementIntegrationTests.testGetMetaData_NoResultSet`

## Testing
- ✅ Local test run:
`PreparedStatementIntegrationTests.testGetMetaData_NoResultSet` passes
- ✅ Metadata now consistent before and after `executeQuery()` for
TIMESTAMP_NTZ columns
- ✅ Both SEA and Thrift paths return "TIMESTAMP" for TIMESTAMP_NTZ
columns

## Related
- Builds on PR databricks#1177 (Fix Arrow field metadata not available for queries
with 0 rows)
- Fixes issue introduced by early metadata extraction in PR databricks#1177
- Maintains backward compatibility with existing behavior

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
…oad parameter (databricks#1183)

## Summary

Add support for disabling CloudFetch via `EnableQueryResultDownload=0`
connection parameter to use inline Arrow results instead.

## Changes

- Add `isCloudFetchEnabled()` method to `IDatabricksConnectionContext`
interface
- Implement the method in `DatabricksConnectionContext` using existing
`EnableQueryResultDownload` parameter
- Update `DatabricksThriftServiceClient` to respect this setting when
making execute requests
- Add unit tests for the new functionality

## Usage

To disable CloudFetch and use inline Arrow results:

```
jdbc:databricks://host:port/default;EnableQueryResultDownload=0;...
```

---------

Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
…lemetry code audit comments- part1 (databricks#1163)

## Description
- 4 things that is improved with respect to telemetry : 
- Common object mapper across telemetry use-case (This is already thread
safe and is expensive to create, i.e., good tor re-use)
- Make`flushIntervalMillis` config same across both telemetry clients
(un-auth and auth)
- Clear connection param cache when connection is closed : this was a
memory leak before
- Rather than creating a scheduledExecutor for each telemetry client, we
share it across a factory.

## Testing
- unit tests

## Additional Notes to the Reviewer
- When the LAST connection to a host is closed, all pending telemetry
events for that host are flushed across all prior connections (since
they all shared the same TelemetryClient). i.e., If you have 5
connections to `host-A`, closing connections 1-4 does nothing (just
decrements refCount). Only when you close connection 5 (the last one)
does the flush occur, sending all accumulated telemetry from all 5
connections.

NO_CHANGELOG=true
## Description

This PR enhances geospatial datatype handling to include SRID (Spatial
Reference System Identifier) information in column type names and fixes
multiple issues related to complex datatype handling across different
result formats.

### Key Changes

1. **Geospatial Type Name Enhancement**
- Column type names now include SRID: `GEOMETRY(4326)` instead of
`GEOMETRY`
   - Applies to both GEOMETRY and GEOGRAPHY types
- Preserves full type information in metadata for better type
identification

2. **SEA Inline Mode Complex Type Fix**
- Fixed issue where complex types (ARRAY, MAP, STRUCT) were not returned
as complex objects in SEA Inline mode (JSON array result format)
- Now properly converts to complex datatype objects when
`EnableComplexDatatypeSupport=true`

3. **Thrift CloudFetch Metadata Enhancement**
- Fixed error when extracting type details (e.g., `INT` from
`ARRAY<INT>`) in Thrift CloudFetch mode
- Enhanced `getColumnInfoFromTColumnDesc()` to use Arrow schema metadata
alongside `TColumnDesc`
- Arrow schema provides complete type information (e.g., `ARRAY<INT>`)
while `TColumnDesc` only contains base type (e.g., `ARRAY`)

4. **Arrow Metadata Extraction**
- Added `DatabricksThriftUtil.getArrowMetadata()` to deserialize Arrow
schema from `TGetResultSetMetadataResp`
- Fixed null arrow metadata issue in `DatabricksResultSet` constructor
for Thrift CloudFetch mode

## Testing

### Unit Tests
- All existing unit tests pass and additional tests are added for new
methods

### Integration Tests
- `GeospatialTests.java` - Comprehensive E2E integration test
  - Tests geospatial types (GEOMETRY and GEOGRAPHY)
  - Validates **24 configuration combinations**:
    - Protocol: Thrift / SEA
    - Serialization: Arrow / Inline
- CloudFetch: Enabled / Disabled (only with Arrow, as CloudFetch
requires Arrow)
    - GeoSpatial Support: Enabled / Disabled
    - Complex Type Support: Enabled / Disabled
  - Validates metadata: column types, type names, class names
  - Validates values: WKT representation, SRID
- Validates behavior when geospatial objects are enabled vs. disabled
(STRING fallback)
  - **All 24 tests pass** ✅

## Additional Notes to the Reviewer

Other required details are mentioned in comments in the diff

---------

Signed-off-by: Sreekanth Vadigi <sreekanth.vadigi@databricks.com>
…cks#1184)

## Summary

Implements proactive prefetching with a sliding window for both Thrift
columnar and inline Arrow results, eliminating blocking at batch
boundaries and improving throughput.

## Key Components

### New Streaming Infrastructure
- **`ThriftStreamingProvider<T>`**: Generic type-safe streaming provider
with background prefetch thread and configurable sliding window
- **`StreamingBatch<T>`**: Type-safe batch container with lifecycle
management and error handling
- **`ThriftResponseProcessor<T>`**: Interface for pluggable response
processors
  - `ColumnarResponseProcessor`: Processes Thrift columnar results
- `InlineArrowResponseProcessor`: Processes inline Arrow results with
schema caching

### Result Implementations
- **`StreamingInlineArrowResult`**: High-throughput streaming
implementation for inline Arrow results with background prefetching
- **`StreamingColumnarResult`**: Streaming implementation for Thrift
columnar results with prefetch

### Supporting Classes
- **`ThriftBatchFetcher`** / **`ThriftBatchFetcherImpl`**: Abstraction
for fetching batches from the Thrift server

<img width="1792" height="1234" alt="streaming inline"
src="https://github.com/user-attachments/assets/66ea9b83-a16b-42d5-9280-cb1fb81dadeb"
/>

## Configuration

| Parameter | Description | Default |
|-----------|-------------|---------|
| `EnableInlineStreaming` | Toggle streaming mode for inline results |
`1` (enabled) |
| `ThriftMaxBatchesInMemory` | Sliding window size (max batches kept in
memory) | `3` |

## Key Features

1. **Background Prefetching**: Dedicated thread fetches batches ahead of
consumption
2. **Sliding Window**: Configurable memory limit prevents unbounded
memory growth
3. **Type Safety**: Generic `ThriftStreamingProvider<T>` eliminates
unsafe casting
4. **Graceful Error Handling**: 
   - Try-catch around resource cleanup to prevent cascading failures
   - Timeout on batch creation wait to prevent indefinite blocking
5. **Comprehensive Logging**: Debug/error logging for troubleshooting

## Testing

- Updated `ExecutionResultFactoryTest` for new factory logic
- Updated `DatabricksThriftServiceClientTest` for CloudFetch control
- Existing integration tests cover streaming behavior

## Usage

Streaming is enabled by default. To disable and use lazy loading
instead:

```
jdbc:databricks://host:port/default;EnableInlineStreaming=0;...
```

To adjust the sliding window size:

```
jdbc:databricks://host:port/default;ThriftMaxBatchesInMemory=5;...
```

---------

Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
…ents (databricks#1186)

## Description

Fixed `IndexOutOfBoundsException` that occurs when executing DDL
statements (e.g., `CREATE DATABASE`) using the Thrift protocol. The bug
manifests when there's a mismatch between the number of Thrift column
descriptors and Arrow schema fields.

### Root Cause

When executing DDL statements, the Databricks server behavior is:
- **Thrift Protocol**: Returns column descriptors including a "Result"
status column (1 column)
- **Arrow Schema**: Returns an empty schema with 0 fields (no actual
data)
- **The Bug**: Code attempted to access `arrowMetadata[0]` without
checking if the list was empty

This mismatch caused `IndexOutOfBoundsException` when the driver tried
to access arrow metadata at index 0 of an empty list.

### Debug Evidence

**TColumnDesc (Thrift)**:
```
Column[0]:
  name: Result
  type: STRING_TYPE
  position: 1
  Full TColumnDesc: TColumnDesc(columnName:Result, typeDesc:TTypeDesc(...), position:1, comment:)
```

**Arrow Schema**:
```
Arrow schema bytes length: 72
Deserialized Arrow schema, field count: 0  ← Empty!
Arrow metadata list: size=0
```

### Changes Made

Added bounds checking in two locations where arrow metadata is accessed:

1. **`ArrowUtil.java:247`** - Used by `StreamingInlineArrowResult`
2. **`DatabricksResultSetMetaData.java:195`** - Used for result set
metadata construction

**Before:**
```java
String columnArrowMetadata =
    arrowMetadata != null ? arrowMetadata.get(columnIndex) : null;
```

**After:**
```java
String columnArrowMetadata =
    arrowMetadata != null && columnIndex < arrowMetadata.size()
        ? arrowMetadata.get(columnIndex)
        : null;
```

## Testing

### Manual Testing

**Test Case**: Execute CREATE DATABASE statement
```java
String sqlQuery = "CREATE DATABASE IF NOT EXISTS hive_metastore.test_db";
boolean hasResultSet = stmt.execute(sqlQuery);
```


**Before Fix**: `IndexOutOfBoundsException: Index 0 out of bounds for
length 0`
**After Fix**: Executes successfully, returns `hasResultSet=false`

## Additional Notes to the Reviewer

NO_CHANGELOG=true

Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
databricks#1181)

## Description
<!-- Provide a brief summary of the changes made and the issue they aim
to address.-->
Modified the logic for enableMultipleCatalogSupport parameter to only
return results when the catalog provided in metadata calls is null or is
equal to the current catalog when the param is disabled. This matches
the behaviour with existing driver.
## Testing
<!-- Describe how the changes have been tested-->
Tested locally 
## Additional Notes to the Reviewer
<!-- Share any additional context or insights that may help the reviewer
understand the changes better. This could include challenges faced,
limitations, or compromises made during the development process.
Also, mention any areas of the code that you would like the reviewer to
focus on specifically. -->
## Description
Implements NonRowcountQueryPrefixes flag to match exiting JDBC driver
behavior. This allows users to specify comma-separated query prefixes
(like INSERT, UPDATE, DELETE) that should return result sets instead of
row counts.

Changes:
- Added NON_ROWCOUNT_QUERY_PREFIXES parameter to DatabricksJdbcUrlParams
- Added getNonRowcountQueryPrefixes() method to connection context
interface and implementation
- Updated shouldReturnResultSet() logic to check configured prefixes
before SQL patterns
- Added 11 comprehensive unit tests covering various scenarios
- Updated NEXT_CHANGELOG.md with feature description

Usage:
NonRowcountQueryPrefixes=INSERT,UPDATE,DELETE,MERGE

## Testing
Tests: All 75 tests pass (64 existing + 11 new)


<!-- Provide a brief summary of the changes made and the issue they aim
to address.-->


<!-- Describe how the changes have been tested-->

## Additional Notes to the Reviewer
<!-- Share any additional context or insights that may help the reviewer
understand the changes better. This could include challenges faced,
limitations, or compromises made during the development process.
Also, mention any areas of the code that you would like the reviewer to
focus on specifically. -->

---------

Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
… new resultset (databricks#1187)

Added support for getClientInfoProperties and getTypeInfo to return a
new resultset and not return the same resultset matching the JDBC spec

## Description
<!-- Provide a brief summary of the changes made and the issue they aim
to address.-->
Added support for getClientInfoProperties and getTypeInfo to return a
new resultset and not return the same resultset matching the JDBC spec
## Testing
<!-- Describe how the changes have been tested-->
Added tests
## Additional Notes to the Reviewer
<!-- Share any additional context or insights that may help the reviewer
understand the changes better. This could include challenges faced,
limitations, or compromises made during the development process.
Also, mention any areas of the code that you would like the reviewer to
focus on specifically. -->

Fixes: databricks#1178
…ricks#1179)

## Description
- Earlier the error telemetry logs only contained 
  - `error_name: "AUTH_ERROR"
stack_trace: "Failed to retrieve the exchanged token"`. 
- With this change, it would be more descriptive - i.e., contain http
error code and reason.
- `Failed to retrieve the exchanged token from OIDC token endpoint. HTTP
request failed by code: 401, status line: HTTP/1.1 401 Unauthorized.
Error message: Invalid credentials.`

## Testing
<!-- Describe how the changes have been tested-->

## Additional Notes to the Reviewer
<!-- Share any additional context or insights that may help the reviewer
understand the changes better. This could include challenges faced,
limitations, or compromises made during the development process.
Also, mention any areas of the code that you would like the reviewer to
focus on specifically. -->
## Description
<!-- Provide a brief summary of the changes made and the issue they aim
to address.-->

## Testing
<!-- Describe how the changes have been tested-->

## Additional Notes to the Reviewer

NO_CHANGELOG=true
## Description
- The request ID is typically in the response header; this PR adds it to
the error message for easier debugging.

## Testing
- Manually testing BYOT flow error scenario 

## Additional Notes to the Reviewer
<!-- Share any additional context or insights that may help the reviewer
understand the changes better. This could include challenges faced,
limitations, or compromises made during the development process.
Also, mention any areas of the code that you would like the reviewer to
focus on specifically. -->
)

## Description
- This fix was made in the [PR
databricks#1135](databricks#1135),
but it did not consider the fetch result timeout. This PR fixes that.

## Testing
- unit tests

## Additional Notes to the Reviewer
- Most of the code changes are propogating errors as `SQLException`, but
the main code change is a small one and is present in
`src/main/java/com/databricks/jdbc/common/util/DatabricksThriftUtil.java`
file

OVERRIDE_FREEZE=true
…autocommit responses

- rollback() now throws DatabricksTransactionException when called in
  auto-commit mode (no active transaction), matching Simba driver behavior
- fetchAutoCommitStateFromServer() now accepts "1"/"0" in addition to
  "true"/"false" from SET AUTOCOMMIT query responses
- Updated E2E tests: rollback without transaction expects exception,
  SET AUTOCOMMIT value check accepts both string and numeric formats
- Added unit tests for new rollback guard and numeric autocommit parsing

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants